Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Limit tracks per user, append "fallback" tracks, check x-forwarded-for header #28

Merged
merged 3 commits into from
Oct 12, 2022

Conversation

girst
Copy link
Contributor

@girst girst commented Oct 14, 2020

This pull does three things at once, apologies in advance. I can submit them individually if you want.

Here's the rationale for each feature/commit (further technical details in the commit messages):

  • Allow reverse-proxying: tunneling Mopidy's HTTP server to my VPS and sticking nginx in front of it, I can let people queue songs by just giving them a domain name to punch into their phones instead of connecting them to my wifi first.
  • Limit how many tracks a user can add back-to-back: Don't let one person ruin the fun for all others by stuffing the playlist with too many songs. Note that it only checks that nobody inserts more than max_tracks (configurable) items consecutively; once a second user queued one song, the first user's limit is reset. Slightly related to Multi user in rotation, one song at a time for each user #23. setting max_tracks to 0 disables the limit (which is the default)
  • allow enqueueing tracks outside of mopidy-party to be used as fallback: Right now, if nobody queues up songs, the music will go silent. This allows 'admins' to queue up tracks with a lower priority by using a different frontend (they will only be played once no more mopidy-party tracks are queued, and new party tracks get inserted before the 'fallback').

While implementing /add, I noticed that you used a GET request for /votes -- I think this should be a POST too. I can add a commit (or another PR) if you want, but I didn't want to swamp you with unsolicited changes too much.

girst added 3 commits October 13, 2020 14:36
Try to use the quasi-standard X-Forwarded-For header to get the IP
address of the client, in case we are being reverse proxied. This is
technically spoofable, but given that the client has access to the
Mopidy WebSocket, they can much easier skip tracks this way:
    mopidy = new Mopidy(); /*wait 1sec*/ mopidy.playback.next();
appending tracks e.g. via mpd will have lower precedence and be used
when user-submitted songs have ran out. new user tracks are inserted
before fallback tracks.
@Lesterpig Lesterpig added hacktoberfest-accepted Accepted for hacktoberfest enhancement labels Oct 29, 2020
@Lesterpig Lesterpig self-requested a review October 29, 2020 10:20
@grasdk
Copy link
Contributor

grasdk commented Oct 10, 2022

Hey. I think this mopidy-plugin has great potential. I like these additions by @girst , though I don't quite understand how the other implementation of "add" (and it's replacement use) allows songs to be queued by other frontend, like mpd with mpd-sima. Anyway, I think the idea is great. Any chance this PR will be merged (it's been lying around for a couple of years in a few days. @Lesterpig still active?

@girst
Copy link
Contributor Author

girst commented Oct 11, 2022 via email

@grasdk
Copy link
Contributor

grasdk commented Oct 11, 2022

Thanks for the explanation. I intend to familiarize myself more with this. I'm interested in the same functionality with the addition that an automated mpd-client like mpd-sima could queue tracks if the queue runs empty (again).

So basically instead of pre-filling a playlist it could be continuously updated if party guests are lazy.

Anyway, more important. Is @Lesterpig around to accept this pull request?

@girst
Copy link
Contributor Author

girst commented Oct 11, 2022 via email

@Lesterpig
Copy link
Owner

Lesterpig self-requested a review 2 years ago

Wow, that is a long inactivity period, sorry for that! 😅
I will do my best to review and test the pull request this week.


def post(self):
# when the last n tracks were added by the same user, abort.
if self.data["queue"] and all([e == self._getip() for e in self.data["queue"]]):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's on purpose but just in case: if there is no song in the queue, and one user has already added max_songs, they cannot add any music anymore.

@Lesterpig Lesterpig merged commit 5bcaa21 into Lesterpig:master Oct 12, 2022
@Lesterpig
Copy link
Owner

Released as 1.1.0, let me know if you have any issue :)

@girst
Copy link
Contributor Author

girst commented Oct 13, 2022 via email

@girst
Copy link
Contributor Author

girst commented Oct 13, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement hacktoberfest-accepted Accepted for hacktoberfest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants